Skip to content

Conversation

@mj-kiwi
Copy link
Contributor

@mj-kiwi mj-kiwi commented Oct 14, 2025

When the confirmation dialog is closed (result is null), the promise should resolve with false to indicate that the user did not grant confirmation. This adds proper handling for the dialog closure scenario and includes a corresponding test case.

Description

Related issues

Fixes:

Manual testing steps

  1. Go to this page...

Screenshots/Recordings

Before

Screen.Recording.2025-10-14.at.2.49.11.PM.mov

After

Screen.Recording.2025-10-14.at.2.44.23.PM.mov

Pre-merge author checklist

Pre-merge reviewer checklist

  • I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
  • I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.

Note

Treat dialog closure as a denied decision, refactor cleanup into a reusable method, and add a test covering the close case.

  • Core (packages/gator-permissions-snap/src/core/confirmation.tsx):
    • Handle snap_dialog closing (result === null) by resolving decision with false and performing partial cleanup (#cleanup(false)).
    • Introduce #cleanup(resolveInterface = true) to unbind handlers and optionally resolve the interface; reuse in button handlers and closeWithError().
    • Ensure #interfaceId is cleared after cleanup on grant/cancel and error paths.
  • Tests (packages/gator-permissions-snap/test/core/confirmation.test.ts):
    • Add test to verify dialog closure resolves with { isConfirmationGranted: false } and unbinds listeners.

Written by Cursor Bugbot for commit f5ef30a. This will update automatically on new commits. Configure here.

…on flow

When the confirmation dialog is closed (result is null), the promise should resolve with false
to indicate that the user did not grant confirmation. This adds proper handling for
the dialog closure scenario and includes a corresponding test case.

This change ensures that the confirmation dialog properly handles the case where
the user closes the dialog without making a decision, returning false instead of
undefined or other unexpected values. The test case verifies this behavior.
@mj-kiwi mj-kiwi requested a review from a team as a code owner October 14, 2025 01:54
cursor[bot]

This comment was marked as outdated.

@mj-kiwi mj-kiwi requested a review from MoMannn October 14, 2025 22:14
@mj-kiwi mj-kiwi requested a review from jeffsmale90 October 21, 2025 03:42
cursor[bot]

This comment was marked as outdated.

Copy link
Contributor

@jeffsmale90 jeffsmale90 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@mj-kiwi mj-kiwi merged commit b44133c into main Oct 27, 2025
16 checks passed
@mj-kiwi mj-kiwi deleted the fix/permission-request-lock branch October 27, 2025 19:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants